Skip to content

change Block to a value struct #53

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

change Block to a value struct #53

wants to merge 1 commit into from

Conversation

gammazero
Copy link
Contributor

The goal is to stop doing two allocations for each block (now it will only allocate the []byte buffer, and pass the cid.Cid, []byte pair by decomposed registers or stack).

This way of changing does not change the syntax for trivial uses of block.Block, so in theory we will have to update only producers of block.Block, not consumers.

Fixes #45

Replaces ipfs/boxo#192

Note: This change will cause a failure here:
https://github.com/ipfs/go-ipld-format/blob/0f7aff00f72e9dea0d9718bc0972e309ba7c3e8d/format.go#L27

@gammazero gammazero mentioned this pull request Aug 6, 2024
Closed
@gammazero
change Block to a value struct
e82c533 
The goal is to stop doing two allocations for each block (now it will only allocate the []byte buffer, and pass the cid.Cid, []byte pair by decomposed registers or stack).

This way of changing does not change the syntax for trivial uses of block.Block, so in theory we will have to update only producers of block.Block, not consumers.

Fixes #45

Replaces ipfs/boxo#192

Note: This change will cause a failure here:
https://github.com/ipfs/go-ipld-format/blob/0f7aff00f72e9dea0d9718bc0972e309ba7c3e8d/format.go#L27
@gammazero gammazero force-pushed the feat/value-block branch from be42814 to e82c533 Compare May 2, 2025 20:41
@gammazero gammazero marked this pull request as ready for review May 2, 2025 20:41
@gammazero
Copy link
Contributor Author

gammazero commented May 23, 2025 •
edited
Loading

This requires many changes in boxo, which passes a traceability.Block around using a Block interface.

There are other places where more functionality is added to a block, or where something like an IPLD Node can be treated as a block, and a Block interface is to pass it.

Removing the Block interface will require many changes. Also, the same interface will reappear in other places to pass block-like objects around as blocks, or code will need to change to not use such an interface.

This PR is good for the purpose of forcing Block to be a struct value where ever possible, without having to change all code to use BasicBlock instead. For convenience, the blocks package could define an Interface type that is really the same as the Block before this PR.

@lidel
Copy link
Member

lidel commented May 27, 2025 •
edited
Loading

Triage notes:

  • wip
  • the interface used in boxo for tracing/tests
  • parking until bitswap spam reduction work is done, then it will be possible to refactor tests/tracing to remove need for interface
  • tbd if we want to keep interface in addition to struct for convenience / avoid code duplication (check lotus use?)

@lidel lidel added the status/blocked Unable to be worked further until needs are met label May 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
status/blocked Unable to be worked further until needs are met
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

valuify blocks.Block
2 participants
@gammazero @lidel